-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Robust Runner #109
Robust Runner #109
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
…obald into maintenance/robust_runner
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
I think I got all comments covered now. Not sure on renaming Ready for the next round, reviewers? ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor one that is not required at all 🥇
Co-authored-by: Eileen Kuehn <[email protected]>
This pull request fixes 1 alert when merging 68b8369 into b33bb8e - view on LGTM.com fixed alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great stuff 🎉 Set it free!
@mschnepf any objections to the new changes? Your last review was invalidated by them. |
@mschnepf I think you accidentally that other PR twice. Can you take another look at this one here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
This PR restructures the concurrency runners to be more robust. Major changes include:
asyncio
fromthreading
async
queue internally which avoids busy looping.asyncio
loop instead of creating a new loopUnittests are down from 15-20s to 2-3s now (which doesn't mean that cobald itself is that much faster.)
Note to reviewers:
The public interface of
register_payload
/run_payload
has stayed intact but the parts used by cobald itself has changed considerably. Since the architecture changed (from threading to asyncio) it is probably not helpful to look just at the diff directly.As a recap of the current architecture:
ServiceRunner
, which has not changed other than some documentation.MetaRunner
, which now runs anasyncio
loop instead of relying onThreadRunner
for concurrency.MetaRunner
starts and monitors severalBaseRunner
instances concurrently.BaseRunner
is the template for actual implementations.